Skip to content

Conversation

@mgree
Copy link
Contributor

@mgree mgree commented Dec 3, 2025

The varchar_to_text cast is functionally a noop, but exists to satisfy the typechecker: varchar(8) and varchar and text are different SQL types.

But they are not different representation types---all of these will be Datum::String at runtime. So: in MIR, we can eliminate this cast.

This PR alters lowering (gated behind the feature flag enable_cast_elimination) to not bother generating varchar_to_text when lowering HIR to MIR.

Alternative implementation to #34348, which fails because the non-repr typechecker gets upset (and because I didn't rewrite the tests yet).

Both PRs will need to have only the repr typechecker running to avoid inconsistencies #34351.

Motivation

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@mgree mgree requested review from a team as code owners December 3, 2025 21:18
@mgree mgree requested a review from ggevay December 3, 2025 21:18
@mgree mgree changed the title alter lowering (with feature flag enable_cast_elimination) to elimi… [repr types] eliminate noop costs (lowering impl) Dec 3, 2025
@ggevay
Copy link
Contributor

ggevay commented Dec 4, 2025

My earlier attempt to do this (#27029) failed in part due to the adapter crate also calling MIR's typ in several places, to get RelationDescs. Has the repr types workstream progressed far enough in the meantime to have eliminated this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants